feat: security hardening#1849
Open
paustint wants to merge 1 commit into
Open
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
This PR implements a set of security hardening changes across the Electron desktop app, the browser extension, server auth/session handling, and sync payload validation, aligning the codebase with the remediation items documented in the new security review.
Changes:
- Desktop: add safe-scheme external URL opening, block cross-origin navigations, and sanitize download filenames with
basename(). - Web extension: remove the
onMessageExternalsurface and tightenpostMessagehandling to same-origin + same-window. - Server/shared: tighten external session revocation scoping, add a max nesting depth guard for sync payloads, and harden geo-ip basic-auth comparison + request caps.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| SECURITY_REVIEW.md | Adds a consolidated security review and remediation status tracking document. |
| libs/types/src/lib/sync/sync.types.ts | Adds max nesting depth protection and caps on hashedKey/orgId in sync schemas. |
| libs/auth/server/src/lib/auth.db.service.ts | Removes sensitive deviceId from logs and scopes external-session revocation to the owning user. |
| apps/jetstream-web-extension/src/extension-scripts/service-worker.ts | Removes onMessageExternal handler to eliminate cross-extension session-fixation surface. |
| apps/jetstream-web-extension/src/extension-scripts/content-auth-script.ts | Restricts message handling to same-origin and event.source === window. |
| apps/jetstream-desktop/src/utils/url.utils.ts | Introduces openExternalSafe() with protocol allow-listing. |
| apps/jetstream-desktop/src/services/protocol.service.ts | Applies basename() to will-download filenames before joining with the download directory. |
| apps/jetstream-desktop/src/services/notification.service.ts | Routes notification actionUrl opening through openExternalSafe(). |
| apps/jetstream-desktop/src/services/file-download.service.ts | Applies basename() to download filenames when omitPrompt is enabled. |
| apps/jetstream-desktop/src/browser/browser.ts | Adds navigation guards (will-navigate/will-redirect) and routes external opens through openExternalSafe(). |
| apps/geo-ip-api/src/main.ts | Adds constant-time-ish credential compare and caps lookup IP array size. |
feat(jetstream-desktop): enhance external URL handling with safe scheme checks fix(jetstream-desktop): ensure file downloads use the correct basename for paths fix(jetstream-desktop): improve notification service to use safe URL opening fix(jetstream-desktop): update protocol service to use basename for download filenames feat(jetstream-desktop): add utility for safe external URL opening fix(web-extension): restrict message event handling to same-origin and window source refactor(web-extension): remove external message listener to enhance security fix(auth.db.service): improve logging and ensure correct session revocation feat(sync.types): add maximum nesting depth check for sync data payloads
paustint
force-pushed
the
sec/harden-070626
branch
from
7937621 to
b855221
Compare
| } catch { | ||
| // Surface the depth-cap breach as a normal Zod validation issue (clean 400) rather than a | ||
| // raw thrown Error that would bubble out of the transform as an unhandled 500. | ||
| ctx.addIssue('Sync record data exceeds the maximum allowed nesting depth'); |
Comment on lines
+55
to
+57
| const usernameMatches = timingSafeStringCompare(username ?? '', ENV.GEO_IP_API_USERNAME ?? ''); | ||
| const passwordMatches = timingSafeStringCompare(password ?? '', ENV.GEO_IP_API_PASSWORD ?? ''); | ||
| if (!usernameMatches || !passwordMatches) { |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
feat(jetstream-desktop): enhance external URL handling with safe scheme checks
fix(jetstream-desktop): ensure file downloads use the correct basename for paths
fix(jetstream-desktop): improve notification service to use safe URL opening
fix(jetstream-desktop): update protocol service to use basename for download filenames
feat(jetstream-desktop): add utility for safe external URL opening
fix(web-extension): restrict message event handling to same-origin and window source
refactor(web-extension): remove external message listener to enhance security
fix(auth.db.service): improve logging and ensure correct session revocation
feat(sync.types): add maximum nesting depth check for sync data payloads